Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Parameterize createAndValidateTransactionBody on ShelleyBasedEra era #378

Merged
merged 11 commits into from
Nov 21, 2023
Merged

Parameterize createAndValidateTransactionBody on ShelleyBasedEra era #378

merged 11 commits into from
Nov 21, 2023

Conversation

Jimbo4350
Copy link
Contributor

@Jimbo4350 Jimbo4350 commented Nov 17, 2023 •
edited
Loading

Changelog

- description: |
    Parameterize `createAndValidateTransactionBody` on `ShelleyBasedEra era`
# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  - breaking       # the API has changed in a breaking way
  # - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - improvement    # QoL changes e.g. refactoring
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

Additional context for the PR goes here. If the PR fixes a particular issue please provide a link to the issue.

How to trust this PR

Highlight important bits of the PR that will make the review faster. If there are commands the reviewer can run to observe the new behavior, describe them.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff

@Jimbo4350 Jimbo4350 force-pushed the jordan/parameterize-createAndValidateTransactionBody-ShelleyBasedEra branch 2 times, most recently from e26e7fb to ee82c03 Compare November 17, 2023 16:01
@Jimbo4350
Parameterize the following functions on ShelleyBasedEra era:
f35c53f 
- scriptLanguageSupportedInEra
- toScriptInEra
- refScriptToShelleyScript
@Jimbo4350 Jimbo4350 force-pushed the jordan/parameterize-createAndValidateTransactionBody-ShelleyBasedEra branch from ee82c03 to ffac1c1 Compare November 17, 2023 16:20
@Jimbo4350 Jimbo4350 changed the title Parameterize createAndValidateTransactioBody on ShelleyBasedEra era Parameterize createAndValidateTransactionBody on ShelleyBasedEra era Nov 17, 2023
@Jimbo4350 Jimbo4350 force-pushed the jordan/parameterize-createAndValidateTransactionBody-ShelleyBasedEra branch 5 times, most recently from 9bd659b to cd8a638 Compare November 17, 2023 21:25
smelc
smelc approved these changes Nov 20, 2023
View reviewed changes
Copy link
Contributor

@smelc smelc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's still draft, but LGTM already

cardano-api/internal/Cardano/Api/TxBody.hs Outdated Show resolved Hide resolved
cardano-api/internal/Cardano/Api/SerialiseLedgerCddl.hs Outdated Show resolved Hide resolved
@Jimbo4350 Jimbo4350 force-pushed the jordan/parameterize-createAndValidateTransactionBody-ShelleyBasedEra branch from cd8a638 to 2e9104f Compare November 20, 2023 16:46
@Jimbo4350 Jimbo4350 marked this pull request as ready for review November 20, 2023 16:46
Jimbo4350
Jimbo4350 commented Nov 20, 2023
View reviewed changes
cardano-api/internal/Cardano/Api/SerialiseLedgerCddl.hs
forEraInEon era
(Left TextEnvelopeCddlErrByronKeyWitnessUnsupported)
(\sbe -> f . InAnyCardanoEra era <$> deserialiseWitnessLedgerCddl sbe teCddl)
AnyShelleyBasedEra era <- cddlTypeToEra ttoken
Copy link
Contributor Author

@Jimbo4350 Jimbo4350 Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Byron witnesses were never supported in the cli: IntersectMBO/cardano-cli@2bcf5df#diff-a38343c1825e6e334224d0a8a1ba78a458652970f00c54f5fb0d040be28f822eL1247

Jimbo4350
Jimbo4350 commented Nov 20, 2023
View reviewed changes
cardano-api/internal/Cardano/Api/SerialiseLedgerCddl.hs
@@ -270,14 +277,12 @@ deserialiseFromTextEnvelopeCddlAnyOf types teCddl =
Left (TextEnvelopeCddlTypeError expectedTypes actualType)

Just (FromCDDLTx ttoken f) -> do
AnyCardanoEra era <- cddlTypeToEra ttoken
f . InAnyCardanoEra era <$> deserialiseTxLedgerCddl era teCddl
AnyShelleyBasedEra era <- cddlTypeToEra ttoken
Copy link
Contributor Author

@Jimbo4350 Jimbo4350 Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Byron txs were read via readByronTx in cardano-cli. readByronTx needs to be moved to cardano-api.

@Jimbo4350 Jimbo4350 force-pushed the jordan/parameterize-createAndValidateTransactionBody-ShelleyBasedEra branch from 2e9104f to 76da97b Compare November 20, 2023 17:03
@Jimbo4350
Implement and expose deserialiseByronTxCddl
5cd5296 
Parameterize deserialiseTxLedgerCddl, deserialiseTx on ShelleyBasedEra
Parameterize constructors of FromSomeTypeCDDL on InAnyShelleyBasedEra
Remove "Witnessed Tx ByronEra" from cddlTypeToEra
@Jimbo4350
Implement and expose getByronTxBody and makeSignedByronTransaction
47e8c1e
@Jimbo4350
Update generators
ce61cdb
@Jimbo4350
Update tests
82789fa
@Jimbo4350 Jimbo4350 force-pushed the jordan/parameterize-createAndValidateTransactionBody-ShelleyBasedEra branch from 76da97b to 82789fa Compare November 20, 2023 19:42
@Jimbo4350 Jimbo4350 added this pull request to the merge queue Nov 21, 2023
Merged via the queue into main with commit eaf4815 Nov 21, 2023
20 checks passed
@Jimbo4350 Jimbo4350 deleted the jordan/parameterize-createAndValidateTransactionBody-ShelleyBasedEra branch November 21, 2023 11:52
@Jimbo4350 Jimbo4350 mentioned this pull request Nov 21, 2023
Closed
3 tasks
newhoggy added a commit that referenced this pull request Mar 11, 2024
@newhoggy
Merge pull request #378 from input-output-hk/ch/disambiguate-treasury…
9ff96ac 
…-withdrawals

Disambiguate treasury withdrawal flags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smelc smelc smelc approved these changes

@carbolymer carbolymer carbolymer approved these changes

@dcoutts dcoutts Awaiting requested review from dcoutts

@erikd erikd Awaiting requested review from erikd

@newhoggy newhoggy Awaiting requested review from newhoggy

@CarlosLopezDeLara CarlosLopezDeLara Awaiting requested review from CarlosLopezDeLara

@disassembler disassembler Awaiting requested review from disassembler

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Jimbo4350 @carbolymer @smelc